-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dashboard: scripts refactored, unit tests enabled #12923
Conversation
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
9429729
to
efa9379
Compare
On the known issue: running I'd use |
@ppitonak Good improvement. Thank you.
And something else. I will check. |
efa9379
to
c88e07d
Compare
@ppitonak I found the problem: you didn't fix the native build 'mvn clean install -Pnative'.
It should be added in this line:
WDYT? |
c88e07d
to
acc644a
Compare
@olexii4 you are right that it's broken now but your solution is not 100% correct. I pushed new version. |
dashboard/pom.xml
Outdated
@@ -197,17 +197,17 @@ | |||
<artifactId>maven-antrun-plugin</artifactId> | |||
<executions> | |||
<execution> | |||
<id>install-yarn</id> | |||
<id>install-deps</id> | |||
<phase>compile</phase> | |||
<goals> | |||
<goal>run</goal> | |||
</goals> | |||
<configuration> | |||
<target> | |||
<!-- install yarn --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not
install yarn
now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix it
So many people, so many minds. Everyone has his own ideas )) |
Signed-off-by: Pavol Pitonak <ppitonak@redhat.com>
acc644a
to
3af1cd8
Compare
@olexii4 I have two guiding principles in this PR
|
Logically. You are quite right. It happens depends on the long story of this project. A long time ago it was a separate repo... I just note you previously what you lost in the simplest way. And you know - I tried to avoid it. Thank you a lot. Good job. |
When will the Jenkins job start? |
If I am not mistaken: 22.00 or 23.00 |
It depends. We do not have a special time. If there will be a free slave... It can be approved after ci-test |
ci-test |
@olexii4 no I cannot run these commands because I'm not part of team and I haven't been white-listed ;) |
ci-build |
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
@ashumilova what will happen now? Will somebody review the results of e2e tests? |
@ppitonak merged, thank you for cleanups and enabling tests. |
What does this PR do?
Running
mvn clean install
did not run unit test. This PRnpm install
only installs dependencies and doesn't run buildpom.xml
so that it uses yarn binary correctlyDockerfile
to callnpm install
just once, not to usenpx
unnecessarily and run unit testsKNOWN ISSUE:
Dockerfile
but the base image already contains yarn, the installed version is not usednpm i yarn
inDockerfile
and (optionally) remove it frompackage.json
OR call yarn binary explicitly using whole path (node_modules/.bin/yarn
) as it's done inpom.xml
What issues does this PR fix or reference?
Release Notes
N/A
Docs PR